Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Overlay deprecation in documentation #7803

Closed
wants to merge 4 commits into from
Closed

Conversation

ahh-docker
Copy link
Contributor

…eed to start here. Also might need to remove the graphic.

Proposed changes

Unreleased project version (optional)

Related issues (optional)

…eed to start here. Also might need to remove the graphic.
@ahh-docker ahh-docker self-assigned this Dec 6, 2018
@ahh-docker
Copy link
Contributor Author

ahh-docker commented Dec 6, 2018

@mark-church @andrewhsu PTAL. Thanks! I know I need to get rid of a graphic for overlay, but I want to make sure the rest flows and I didn't remove anything that didn't need to be removed.

@mark-church
Copy link

Adding in @mataneja in my place to review since he is the PM for engine (and overlay driver)

@GordonTheTurtle
Copy link

GordonTheTurtle commented Dec 6, 2018

Deploy preview for docsdocker ready!

Built with commit 111ae39

https://deploy-preview-7803--docsdocker.netlify.com

@ahh-docker
Copy link
Contributor Author

Thanks @mark-church! I've added @mataneja to the reviewers!

@mataneja
Copy link

Overall LGTM. @andrewhsu / @thaJeztah please review.

storage/storagedriver/index.md Outdated Show resolved Hide resolved
storage/storagedriver/index.md Outdated Show resolved Hide resolved
storage/storagedriver/overlayfs-driver.md Outdated Show resolved Hide resolved
> For more information about differences between `overlay` vs `overlay2`, check
> [Docker storage drivers](select-storage-driver.md).

The `overlay2` driver only works with two layers. This means that multi-layered
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is about the overlay driver (which only can work with a single "lowerdir". The overlay2 driver does not have this limitation, and doesn't have to do the hardlink trick to workaround the "single lowerdir" limitation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah can you please explain to me what edits need to be done? thanks.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @thaJeztah means is this paragraph describes the details of overlay and is not applicable to overlay2.

storage/storagedriver/overlayfs-driver.md Outdated Show resolved Hide resolved
less performant than `overlayfs` on initial read, because it must look
through more layers, but it caches the results so this is only a small
penalty.
for files in images with many layers. This advantage applies to the `overlay2`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This advantage actually was for the overlay driver (as explained in the removed part); overlay2 has a performance penalty on first read (as it has multiple lowerdirs), but once cached, that performance penalty no longer applies.

@@ -490,14 +442,6 @@ Both `overlay2` and `overlay` drivers are more performant than `aufs` and
far larger latencies if searching through many AUFS layers. `overlay2` supports
multiple layers as well, but mitigates any performance hit with caching.

- **Inode limits**. Use of the `overlay` storage driver can cause excessive
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wondering if we should add a page about the overlay driver and capture this information there.

This issue has been the major pain-point for the legacy overlay driver; people ran out of inodes (resulting in "no space on device") whereas actually there was enough space left, only not enough inodes

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah Since we're deprecating the overlay driver, why should we keep documentation on the older driver? If you want to have something on why we are deprecating it, I'm for it.

@andrewhsu What are your thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have that info because even if the driver is deprecated, there are still people using it (and will be, for at least a few years). Having a doc to help them out is good, especially since we already have that info written -- it just need to be moved into a separate doc having a preamble/disclaimer like "Note overlay is deprecated and should not be used; the information below is kept here mostly for historical purposes" or so.

@@ -76,7 +76,7 @@ the Docker edition you use.

In addition, Docker does not recommend any configuration that requires you to
disable security features of your operating system, such as the need to disable
`selinux` if you use the `overlay` or `overlay2` driver on CentOS.
`selinux` if you use the `overlay2` driver on CentOS.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to double check selinux; IIRC, SELinux is now supported with overlay/overlay2, but there may be some limitations still

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah Can you please let me know what the limitations are?

@@ -342,23 +298,23 @@ hard links to the data that is shared with lower layers. This allows for
efficient use of disk space.

```bash
$ ls -i /var/lib/docker/overlay/38f3ed2eac129654acef11c32670b534670c3a06e483fce313d72e3e0a15baa8/root/bin/ls
$ ls -i /var/lib/docker/overlay2/38f3ed2eac129654acef11c32670b534670c3a06e483fce313d72e3e0a15baa8/root/bin/ls
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll have to revisit this section (haven't had time yet to check if it's still current)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@thaJeztah please let me know. Thanks.

@ahh-docker
Copy link
Contributor Author

@JustinINevill This is still in technical review.

Copy link
Contributor

@kolyshkin kolyshkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Left a few comments; also would be great to squash the commits for the sake of easier review.

Storage drivers create data in the writable layer of your container. The files won't
be persisted after the container is deleted, and both read and write speeds are low.

Docker recommends Overlay2 for the storage driver.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to have it Capitalized, i.e.
Overlay2 -> overlay2


> **Note**: The `overlay` driver is deprecated in Docker for 18.09 and later.
> Docker recommends `overlay2` for its storage driver.
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The information about what kernel is required for overlay2 ("you need version 4.0 or higher of the Linux kernel"...) is useful and should not be removed.

> For more information about differences between `overlay` vs `overlay2`, check
> [Docker storage drivers](select-storage-driver.md).

The `overlay2` driver only works with two layers. This means that multi-layered
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @thaJeztah means is this paragraph describes the details of overlay and is not applicable to overlay2.

storage/storagedriver/overlayfs-driver.md Outdated Show resolved Hide resolved
@@ -490,14 +442,6 @@ Both `overlay2` and `overlay` drivers are more performant than `aufs` and
far larger latencies if searching through many AUFS layers. `overlay2` supports
multiple layers as well, but mitigates any performance hit with caching.

- **Inode limits**. Use of the `overlay` storage driver can cause excessive
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should have that info because even if the driver is deprecated, there are still people using it (and will be, for at least a few years). Having a doc to help them out is good, especially since we already have that info written -- it just need to be moved into a separate doc having a preamble/disclaimer like "Note overlay is deprecated and should not be used; the information below is kept here mostly for historical purposes" or so.

@ahh-docker
Copy link
Contributor Author

@kolyshkin @thaJeztah Can I get together with the two of you and get this fixed and merged? I'd like to get it out of my queue, and it's gotten quite unweidy.. Thanks. :)

@traci-morrison
Copy link
Contributor

traci-morrison commented Dec 11, 2019

@thaJeztah Hi Sebastiaan, are the changes in this PR still valid? Thanks!

@traci-morrison traci-morrison requested review from thaJeztah and removed request for mark-church December 12, 2019 16:00
@traci-morrison
Copy link
Contributor

@thaJeztah The Infrastructure team is copying our repos over to Mirantis this week. Any open PRs and issues will be lost, so please do what you can to close or merge this PR before Thursday. Thank you!

@usha-mandya
Copy link
Member

@thaJeztah Could you let me know whether this PR still valid for Docker docs?

@dockertopia dockertopia added the area/networking Relates to anything around networking label Jul 8, 2022
@docker-robott
Copy link
Collaborator

Thanks for the pull request. We'd like to make our product docs better, but haven’t been able to review all the suggestions.
As our docs have also diverged, we do not have the bandwidth to review and rebase old pull requests.

If the updates are still relevant, review our contribution guidelines and rebase your pull request against the latest version of the docs, then mark it as fresh with a /remove-lifecycle stale comment.
If not, this pull request will be closed in 30 days. This helps our maintainers focus on the active pull requests.

Prevent pull requests from auto-closing with a /lifecycle frozen comment.

/lifecycle stale

@crazy-max crazy-max deleted the overlay-deprecation-7439 branch January 14, 2023 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/networking Relates to anything around networking lifecycle/stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.